feat: `atrium-repo` #272

DrChat · 2024-12-23T20:14:30Z

This builds on str4d's work here: #168.

Big notes:

Complete merkle search tree implementation (but still lacking fuzz tests)
Complete Repository implementation with support for basic CRUD operations
Added a new pair of traits for block storage: AsyncBlockStoreRead and AsyncBlockStoreWrite.
- In-memory storage backend (primarily intended for testing)
- CAR storage (supporting both reading and writing)
- Users can implement other storage sources, like Azure-backed block storage.

Merkle Search Tree
- Parse Node
  - Read Node from bytes
  - Verify depth and sort order
  - Limit the number of TreeEntrys per Node to a statistically unlikely maximum length.
  - Consider limiting the overall depth of the repo, or other parameters, to prevent more sophisticated key mining attacks.
- Locate key within node
- Locate keys within node with a given prefix
- Add entry
- Edit entry
- Delete entry
Repository
- Load from a CAR file
- Load from a firehose record
- Read/write records inside of a repository
Storage
- CAR files
  - Reading CAR files
  - Verify completeness of the repository structure.
  - Robustness to both duplication and de-duplication of blocks.
  - Ignore any unnecessary or unlinked blocks.
- Commits from firehose records

DrChat · 2025-01-01T20:11:19Z

atrium-repo/src/mst.rs

+        loop {
+            let node = Node::read_from(&mut bs, node_cid).await?;
+            if !seen.insert(node_cid) {
+                // This CID was already seen. There is a cycle in the graph.


Is it even possible to serialize a graph with a cycle?
The two nodes would point at each other, and by definition it should be impossible to compute their CIDs if there is a cycle.

So I actually don't think this is possible, which means the extra validation logic here is probably optional and very unlikely to be hit in practice.
To err on the side of safety though, I think it would be best to leave this in-place.

DrChat · 2025-01-18T22:24:33Z

I think the PR is in a pretty good state for a MVP.
My time is limited so I won't be able to quickly iterate on the functionality for creating new commits for a repository. Still planning on it, but the work here can be pushed up in the meantime :)

erlend-sh · 2025-01-19T12:29:03Z

Pinging @str4d since this builds on their prior work.

sugyan

I have written several comments on areas unrelated to the internal logic. There are also a couple of points from clippy that I hope you can correct as well.

sugyan · 2025-01-19T13:50:01Z

.gitignore

@@ -12,5 +12,8 @@ target/
 # These are backup files generated by rustfmt
 **/*.rs.bk

+# VSCode settings
+/.vscode


I do not want to include anything related to a specific editor.

I added this line because VSCode annoyingly creates this folder automatically (likely due to the Copilot extension being broken and populating a settings.json).
If it isn't present, it's almost certain that I or someone else may accidentally commit this folder.

If you don't want to have a line that explicitly mentions VSCode, another alternative you could consider is changing this .gitignore file to an allowlist.
Here's an example from one of my other projects.

Each individual's settings related to the editor should have their own global ignore settings, and I don't want to include changes on the repository side for that.

According to man gitignore:

Patterns which a user wants Git to ignore in all situations (e.g., backup or temporary files generated by the user’s editor of choice) generally go into a file specified by core.excludesFile in the user’s ~/.gitconfig. Its default value is $XDG_CONFIG_HOME/git/ignore. If $XDG_CONFIG_HOME is either not set or empty, $HOME/.config/git/ignore is used instead.

I believe that editor related ignore settings should be written there.

Could I still persuade you to reconsider? :)
This is commonly done in many projects - for example, Rust itself has a .gitignore file checked in with this line present.

I do work with some projects that are exceptions to this rule and feel like a global setting would be more cumbersome than helpful.

sugyan · 2025-01-19T13:51:19Z

Cargo.toml

-    "examples/concurrent",
-    "examples/firehose",
-    "examples/video",
+    # "examples/concurrent",


Is there a reason why these are no longer excluded?

Ah - I had forgotten about this.
I changed this because the examples were written against the last released version of atrium rather than the version present in the repository.
I needed to update the firehose example to incorporate the changes I have made here, and chose to change its dependencies to point at the crates in this repository.

The decision to have examples demonstrate the last public release seems to be an unusual one. I haven't ever seen this be done for any other Rust library.
Any thoughts about changing this?

Indeed. The current policy of using the latest release version does not allow us to use newly added packages.

We've discussed this before and made it the way it is now,
#100 (comment)

But maybe we should reconsider.

Indeed - I think it would be wise to reconsider :)

One potential alternative to allow people to find examples for the previously released version would be to create git tags upon release and instruct users to look for the corresponding tag.
It actually looks like release-plz is already creating these tags for you (though appears to be doing so per-crate).

atrium-repo/Cargo.toml

Cargo.toml

sugyan · 2025-01-19T13:58:09Z

atrium-repo/README.md

+[![](https://img.shields.io/crates/v/atrium-repo)](https://crates.io/crates/atrium-repo)
+[![](https://img.shields.io/docsrs/atrium-repo)](https://docs.rs/atrium-repo)
+[![](https://img.shields.io/crates/l/atrium-repo)](https://github.com/sugyan/atrium/blob/main/LICENSE)
+[![Rust](https://github.com/sugyan/atrium/actions/workflows/api.yml/badge.svg?branch=main)](https://github.com/sugyan/atrium/actions/workflows/api.yml)


Since this package also contains tests, you should also add workflows/repo.yml and this should be a link to it.

…t firehose records

Why: This function is a footgun that will likely be misused. Deleting a block is not typically safe because it could have multiple incoming references (and thus deletion converts those into dangling references). The only way to safely delete a block would be to wholistically enumerate an archive file and collect a list of all referenced blocks.

DrChat · 2025-02-08T16:37:36Z

Note: It looks like there's a pretty exhaustive MST test suite here that could probably be leveraged to validate this implementation.
https://github.com/DavidBuchanan314/mst-test-suite

DrChat marked this pull request as draft December 23, 2024 20:14

DrChat force-pushed the repo branch 6 times, most recently from 3b84377 to f8e988e Compare December 26, 2024 21:08

DrChat force-pushed the repo branch 2 times, most recently from f736ccc to b230066 Compare January 1, 2025 19:20

DrChat commented Jan 1, 2025

View reviewed changes

DrChat force-pushed the repo branch 2 times, most recently from 6db3530 to 60adc4a Compare January 18, 2025 21:58

DrChat marked this pull request as ready for review January 18, 2025 22:24

sugyan self-requested a review January 19, 2025 13:48

sugyan requested changes Jan 19, 2025

View reviewed changes

DrChat force-pushed the repo branch 4 times, most recently from 690f6b1 to 5f55fc5 Compare January 31, 2025 01:03

str4d and others added 9 commits February 1, 2025 11:51

chore: Create empty atrium-repo library crate

bfb8c17

repo: Logic for parsing and traversing MST nodes

682f0e4

repo: Indexed reader for CAR files

e33db08

repo: APIs to fetch keys, collections, and records from a repository

cb49514

Significant refactor; implement basic MST update logic

1d4c101

Full MST implementation

d633238

More refactoring :)

5343362

Use traverse for split_subtree

e9532bb

Fix a few lingering bugs in node deletion

4e2f052

DrChat added 26 commits February 1, 2025 11:52

Fix repo tests

bee8f13

More docs

f7fa5a9

Simplify Tree::keys

63eb5e5

Return keys from Tree::keys in lexicographic order

daa6165

Create a high-level Commit helper struct

381c304

Add new Tree::entries function to return key/value tuples

16db1bd

Add documentation clarifying that tree iteration does not work agains…

ce8e32b

…t firehose records

Whoops :)

d281bd6

Expose hashing algorithm to AsyncBlockStoreWrite::write_block

973b06c

Forgot to commit this import :(

caa3681

Gracefully handle multi-writes to CarStore

cb7222c

Rename Repository::new to open

b4e8a99

Outline algos::compute_depth

5ce2e07

Correct the docs for Node::find_ge

f7b910e

Panic if we attempt to serialize a Node with two adjacent tree entries

9665d2b

Misc. improvements

91bee28

More Tree documentation

f01c0ba

Fix a bug when writing out a new CAR block

5cfc9c3

API updates; Implement CarStore::create

4a921c1

Update firehose example to remove CID compat

50ca94d

Move all algorithms to algos module

baffd15

Fix warnings

5f9edb4

clippy + fmt

7612064

Full CRUD functionality for Repository

5da91d0

Add a basic differencing blockstore layer

cb8e8c0

DrChat force-pushed the repo branch from 5f55fc5 to cb8e8c0 Compare February 1, 2025 17:52

DrChat added 2 commits February 1, 2025 11:53

Remove async-trait dep

ef20156

Add my name to atrium-repo authors

bbd9cd1

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `atrium-repo` #272

feat: `atrium-repo` #272

DrChat commented Dec 23, 2024 •

edited

Loading

DrChat Jan 1, 2025

DrChat Feb 1, 2025

DrChat commented Jan 18, 2025

erlend-sh commented Jan 19, 2025 •

edited

Loading

sugyan left a comment

sugyan Jan 19, 2025

DrChat Jan 19, 2025

sugyan Jan 20, 2025

DrChat Jan 24, 2025

sugyan Jan 19, 2025

DrChat Jan 19, 2025

sugyan Jan 20, 2025

DrChat Jan 24, 2025

sugyan Jan 19, 2025

DrChat commented Feb 8, 2025

feat: atrium-repo #272

Are you sure you want to change the base?

feat: atrium-repo #272

Conversation

DrChat commented Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrChat commented Jan 18, 2025

erlend-sh commented Jan 19, 2025 • edited Loading

sugyan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrChat commented Feb 8, 2025

feat: `atrium-repo` #272

feat: `atrium-repo` #272

DrChat commented Dec 23, 2024 •

edited

Loading

erlend-sh commented Jan 19, 2025 •

edited

Loading